Skip to content

Conversation

martindurant
Copy link
Member

No description provided.

@@ -306,6 +306,9 @@ def makedirs(self, path, exist_ok=False):
"""
pass # not necessary to implement, may not have directories

def bulk_makedirs(self, path, **kw):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need a special function here instead of modifying the makedirs function?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had imagined it like cp/cp_file

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't we just override makedirs to no check if the dirrectory exists for object stores since we directories aren't really a concept there?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_makedirs is overidden by implementations already, so every implementation would have to make the list-of-paths check or do some sort of super()

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am still confused is this suppose to overridden by the implementation? Are there are implementations that override it currently?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No there are none, but s3fs would be the first, since we don't want to check the existence of every directory, only the root bucket, or try to make the bucket multiple times.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you can think of a simpler way to do it, happy to hear!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants